Minify Helm chart values JSON schema#595
Conversation
Remove unused functions Add default switch clause
|
Issues linked to changelog: |
| } | ||
|
|
||
| return indentJson(string(jsonSchema)) | ||
| return string(jsonSchema) |
There was a problem hiding this comment.
This is the only actual change.
|
looks like some tests are failing |
|
Would it be possible to bump helm in CI to v3.17.3? That would difinitively prove GME can be installed with helm v3.17.3 with the minified json. Also, while you're at it, could you bump the helm dependency in go.mod to |
| } | ||
|
|
||
| // minifyJSON is a helper function for tests to remove whitespace from JSON strings | ||
| func minifyJSON(jsonStr string) (string, error) { |
There was a problem hiding this comment.
json.Compact might work here https://pkg.go.dev/encoding/json#Compact
There was a problem hiding this comment.
Thanks for the pointer, I wasn't aware of that function! Just tested and it works like a charm 🎉
Sodman
left a comment
There was a problem hiding this comment.
LGTM although I agree with Aaron's feedback that we should bump ourselves out of CVEs with this change too. Also agree with Jenny that if there's a stdlib way to do the same thing, I'd prefer that over a DIY solution.
@birkland These considerations totally make sense and I was planning on bumping the dependency, but we need to do that in the GME repo. We don't have any Go dependency on helm here. Did you perchance think this PR is in the GME repo? |
* Don't prettify chart values JSON schema * Changelog * Make linter happy Remove unused functions Add default switch clause * Move changelog * Codegen * Update unit tests assertions * Tests: use `json.Compact` to minify * Un-focus test * Remove unused function # Conflicts: # codegen/test/chart/values.schema.json
….x (#596) * Update SetClusterName and GetClusterName to use generateName instead of annotations (#593) * add backwards compatibility in skv2 * add changelog * update skv2 to not fallback to annotations * try backwards compatibility fallback on gets, but not sets * clean up skv2 unit tests * fix changelog * fix issuelink * address nit * move changelog * move changelog into 0.36.7 since it was not cut yet * Revert "move changelog into 0.36.7 since it was not cut yet" This reverts commit 341c6c2. * Minify Helm chart values JSON schema (#595) * Don't prettify chart values JSON schema * Changelog * Make linter happy Remove unused functions Add default switch clause * Move changelog * Codegen * Update unit tests assertions * Tests: use `json.Compact` to minify * Un-focus test * Remove unused function # Conflicts: # codegen/test/chart/values.schema.json * move minify helm changelog * merge changelog files * fix codegen
* Don't prettify chart values JSON schema * Changelog * Make linter happy Remove unused functions Add default switch clause * Move changelog * Codegen * Update unit tests assertions * Tests: use `json.Compact` to minify * Un-focus test * Remove unused function (cherry picked from commit ab8ad1c)
* Minify Helm chart values JSON schema (#595) * Don't prettify chart values JSON schema * Changelog * Make linter happy Remove unused functions Add default switch clause * Move changelog * Codegen * Update unit tests assertions * Tests: use `json.Compact` to minify * Un-focus test * Remove unused function (cherry picked from commit ab8ad1c) * Move changelog
Motivation
With this change, the
values.schema.jsonfile generated as part of the Helm chart generation is no longer prettified. This drastically reduces the size of the file and minimizes the changes of the new maximum file size limit introduced by Helm inv3.17.3. There are no functional changes. Human users can still prettify the file using external tools (e.g.jq) if necessary.Testing
You can test that this fixes the GME issue by following the instructions on https://github.com/solo-io/gloo-mesh-enterprise/pull/20443.